DYN-8334 Node Cluster Visual Preparation Work #15865
Conversation
…DynamoDS/Dynamo into NodeClusterPlacementPerformance
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
See the ticket for this pull request: https://jira.autodesk.com/browse/DYN-8334
|
Does this PR actually stop these nodes from being executed somehow? I don't think just disabling their preview bubbles is doing that or is that work WIP? |
| //Get those modified nodes that are not frozen | ||
| foreach (var node in workspace.Nodes.Where(n => n.IsModified && !n.IsFrozen)) | ||
| //Get those modified nodes that are not frozen or preview states | ||
| foreach (var node in workspace.Nodes.Where(n => n.IsModified && !n.IsFrozen && !n.IsPreview)) |
There was a problem hiding this comment.
@mjkkirschner I think this line exclude the nodes from execution, right?
There was a problem hiding this comment.
How about if we modify the Nodes property so that we can be sure preview nodes are ignored in all contexts.
And I guess we'll only account for them in all mechanisms engaged during preview placement.
There was a problem hiding this comment.
I'm not sure - I would take a look at all the places is IsFrozen is referenced - I kind of remember changes had to be made at the AST level to avoid the codegen step for frozen nodes - but it was a really long time ago.
There was a problem hiding this comment.
I think currently I covered all the place IsFrozon is referenced and relevant
| writer.WritePropertyName("Nodes"); | ||
| serializer.Serialize(writer, ws.Nodes); | ||
| serializer.Serialize(writer, ws.Nodes.Where(x => x.IsTransient != true); |
There was a problem hiding this comment.
This would make sure when node/cluster preview is showing and user press save or shortcut, they dont end up into the dyn
|
@BogdanZavu @chubakueno I believe the PR is ready for review |
| @@ -2435,6 +2435,8 @@ Dynamo.ViewModels.NodeViewModel.IsDisplayingLabels.set -> void | |||
| Dynamo.ViewModels.NodeViewModel.IsExperimental.get -> bool | |||
| Dynamo.ViewModels.NodeViewModel.IsFrozen.get -> bool | |||
| Dynamo.ViewModels.NodeViewModel.IsFrozen.set -> void | |||
| Dynamo.ViewModels.NodeViewModel.IsTransient.get -> bool | |||
There was a problem hiding this comment.
Is this something that we actually want to be public ? I think it's ok for now and we can think/debate later on this subject.
There was a problem hiding this comment.
Good point. Let me see if I can make it internal for now
There was a problem hiding this comment.
Looks like not, the property is bind to UI layer and needs to be public, at least NodeViewModel layer, I can make the NodeModel layer internal
There was a problem hiding this comment.
If I remember right we can have something public and change our mind a little later while in master ? Or no
|
LGTM! The Transient state is looking really nice! |
Yup, thanks for the review, waiting for PR checks |
Purpose
This PR is to prepare the visual basics of node cluster per DYN-8334. The baselines are:
IsPreviewstate of nodesTODO:
The following examples are using 3 watch node to mock a typical cluster with 3 nodes:
zoomed in:
zoomed out:
Gif:
Declarations
Check these if you believe they are true
*.resxfilesRelease Notes
This PR is to prepare the visual basics of node cluster
Reviewers
@BogdanZavu @chubakueno
FYIs
@johnpierson @Jingyi-Wen